Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow reordering questions using the keyboard #1532

Merged
merged 3 commits into from
Oct 24, 2023
Merged

Allow reordering questions using the keyboard #1532

merged 3 commits into from
Oct 24, 2023

Conversation

susnux
Copy link
Collaborator

@susnux susnux commented Feb 22, 2023

Part 1: Adding missing aria role

Resolves: #1263
divs require a role when aria-label is set (see context in linked issue)

Part 2: Add buttons

Resolves: #317
Added buttons for changing the order. Buttons can be focused and triggered using the keyboard.
This also preserves the drag and drop handling, but also allows using buttons or the keyboard.

The buttons are only shown if hovered / focused, but they are always accessible for screen readers.

If a button is clicked / triggered the focus is kept on the button (the vanishing focus ring in the following video happens only because disabled buttons from @nextcloud/vue do not have a visual focus ring)

vokoscreenNG-2023-02-22_02-26-21.mp4

@susnux susnux added enhancement New feature or request design Related to the design 3. to review Waiting for reviews labels Feb 22, 2023
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice enhancement! :) Makes it very obvious and accessible how to reorder, good stuff.

Only one thing: When reordering via these buttons, whether it be via keyboard or mouse click, it would be nice to have a quick animation of the movement using var(--animation-quick) or var(--animation-slow) depending on what looks better. :)

@susnux
Copy link
Collaborator Author

susnux commented Feb 22, 2023

Very nice enhancement! :) Makes it very obvious and accessible how to reorder, good stuff.

Thank you :)

Only one thing: When reordering via these buttons, whether it be via keyboard or mouse click, it would be nice to have a quick animation of the movement using var(--animation-quick) or var(--animation-slow) depending on what looks better. :)

Done. Added the animation, I choose slow because quick for long questions looks too "jumpy".

vokoscreenNG-2023-02-22_15-46-53.mp4

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really amazing now @susnux, great work! :)

@Chartman123
Copy link
Collaborator

@susnux I get lots of errors in the browser console like this:

[Vue warn]: Invalid prop: type check failed for prop "setReturnFocus". Expected Object, String, Function, Boolean, got HTMLButtonElement 

found in

---> <NcPopover>
       <NcActions>
         <RouterLink>
           <NcListItem>
             <AppNavigationForm> at src/components/AppNavigationForm.vue
               <NcAppNavigation>
                 <NcContent>
                   <Forms> at src/Forms.vue
                     <Root> 2 forms-main.js:134698:21

The lower part of the message varies...

And at least for me the focus ring always stays on the ⬇️ button after clicking on it with the mouse. The ⬆️ button doesn't show the ring.

@susnux
Copy link
Collaborator Author

susnux commented Feb 23, 2023

@Chartman123 the errors should be fixed with nextcloud Vue 7.7.0:
nextcloud-libraries/nextcloud-vue#3803

@Chartman123
Copy link
Collaborator

@susnux: With nc-vue 7.7 the errors are gone. But now I found some new ones: canMoveUp and canMoveDown are missing in the view mode and thus throwing errors.

[Vue warn]: Missing required prop: "canMoveUp"

found in

---> <Question> at src/components/Questions/Question.vue
       <QuestionDropdown> at src/components/Questions/QuestionDropdown.vue
         <NcAppContent>
           <Submit> at src/views/Submit.vue
             <NcContent>
               <Forms> at src/Forms.vue
                 <Root>

@susnux
Copy link
Collaborator Author

susnux commented Feb 28, 2023

With nc-vue 7.7 the errors are gone. But now I found some new ones: canMoveUp and canMoveDown are missing in the view mode and thus throwing errors.

@Chartman123 those issues are resolved. And the focus issue is also resolved.

@Chartman123
Copy link
Collaborator

With nc-vue 7.7 the errors are gone. But now I found some new ones: canMoveUp and canMoveDown are missing in the view mode and thus throwing errors.

@Chartman123 those issues are resolved. And the focus issue is also resolved.

Nice :) I will check it this evening

src/components/Questions/QuestionDate.vue Show resolved Hide resolved
src/components/Questions/Question.vue Outdated Show resolved Hide resolved
src/views/Create.vue Outdated Show resolved Hide resolved
@susnux susnux force-pushed the fix/drag-a11y branch 2 times, most recently from f0b1eb3 to b08192c Compare March 14, 2023 23:21
@Chartman123 Chartman123 added this to the 3.4 milestone Jun 26, 2023
@susnux susnux mentioned this pull request Jul 28, 2023
4 tasks
@susnux susnux force-pushed the fix/drag-a11y branch 3 times, most recently from cf128ed to 655c4ed Compare October 16, 2023 22:50
@susnux
Copy link
Collaborator Author

susnux commented Oct 16, 2023

  • Rebased
  • Fixed a11y issues
  • Fixed transition for both: Keyboard and mouse sorting
vokoscreenNG-2023-10-17_00-44-37.mp4

@susnux susnux merged commit 431914e into main Oct 24, 2023
22 checks passed
@susnux susnux deleted the fix/drag-a11y branch October 24, 2023 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews design Related to the design enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lighthouse accessibility failures Reorder question via arrows
5 participants